Skip to content

fix(landing-nav): scroll to top on route change in shared shells#4676

Merged
waleedlatif1 merged 7 commits into
stagingfrom
waleedlatif1/nav-scroll-restoration
May 20, 2026
Merged

fix(landing-nav): scroll to top on route change in shared shells#4676
waleedlatif1 merged 7 commits into
stagingfrom
waleedlatif1/nav-scroll-restoration

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

Summary

  • Mount a <ScrollToTop /> client component in the blog, integrations, and models shared layouts to reset scroll on usePathname change
  • Skip the reset on popstate-driven navigation so browser back/forward scroll restoration is preserved
  • Works around documented Next.js App Router behavior where the framework only scrolls the new route segment into view, often resulting in no scroll inside shared layouts (Clicking a Link does't scroll to top on the next page vercel/next.js#64435)

Type of Change

  • Bug fix

Testing

Tested manually — recommend smoke testing listing → detail and detail → listing on blog, integrations, and models, plus browser back/forward

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link
Copy Markdown

vercel Bot commented May 20, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped May 20, 2026 7:26pm

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 20, 2026

PR Summary

Low Risk
Low risk UI behavior change limited to landing-page shared layouts, though it could subtly affect expected scroll restoration on navigation.

Overview
Adds a new client-only ScrollToTop component that calls window.scrollTo(0, 0) whenever the App Router pathname changes (skipping hash-anchor navigations).

Mounts this component in the shared layouts for blog, integrations, and models so route changes within these shells reliably reset scroll position.

Reviewed by Cursor Bugbot for commit 7e926fe. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 20, 2026

Greptile Summary

This PR introduces a ScrollToTop client component and mounts it in the blog, integrations, and models shared layouts to work around the documented Next.js App Router behavior (vercel/next.js#64435) where the framework only scrolls the new route segment into view, leaving scroll position unchanged inside shared layouts.

  • scroll-to-top.tsx: Minimal useEffect on usePathname that calls window.scrollTo(0, 0); correctly guards against overriding hash-anchor scrolls with if (window.location.hash) return.
  • Three layouts (blog, integrations/(shell), models/(shell)): Each mounts <ScrollToTop /> as a null-rendering client island inside the server layout, using the correct @/ alias import path.

Confidence Score: 5/5

Safe to merge — the change is a small, well-scoped client component that renders nothing and only calls window.scrollTo on route changes.

The component is minimal and isolated: it uses standard Next.js hooks, correctly short-circuits on hash anchors, and the three layout files each add a single import and one JSX element with no logic changes. No data, auth, or rendering paths are affected.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/app/(landing)/components/scroll-to-top.tsx New ScrollToTop client component that resets scroll on pathname change and correctly skips the reset when a hash anchor is present.
apps/sim/app/(landing)/blog/layout.tsx Mounts <ScrollToTop /> inside the blog shared layout; import and placement are correct.
apps/sim/app/(landing)/integrations/(shell)/layout.tsx Mounts <ScrollToTop /> inside the integrations shell layout; import and placement are correct.
apps/sim/app/(landing)/models/(shell)/layout.tsx Mounts <ScrollToTop /> inside the models shell layout; import and placement are correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User navigates to new pathname] --> B[usePathname change triggers useEffect]
    B --> C{window.location.hash\nnon-empty?}
    C -- Yes --> D[Return early\nBrowser handles anchor scroll natively]
    C -- No --> E[window.scrollTo 0, 0\nReset to top of page]
    E --> F[User sees page from the top]
    D --> G[User lands at hash target]
Loading

Reviews (8): Last reviewed commit: "refactor(landing-nav): simplify scroll-t..." | Re-trigger Greptile

Comment thread apps/sim/app/(landing)/components/scroll-to-top.tsx Outdated
Comment thread apps/sim/app/(landing)/components/scroll-to-top.tsx
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/app/(landing)/components/scroll-to-top.tsx
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/app/(landing)/components/scroll-to-top.tsx Outdated
Comment thread apps/sim/app/(landing)/components/scroll-to-top.tsx
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/app/(landing)/components/scroll-to-top.tsx
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/app/(landing)/components/scroll-to-top.tsx Outdated
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 1f67317. Configure here.

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/app/(landing)/components/scroll-to-top.tsx
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 7e926fe. Configure here.

@waleedlatif1 waleedlatif1 merged commit c381550 into staging May 20, 2026
14 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/nav-scroll-restoration branch May 20, 2026 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant